Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove assumption user ID is 1000 #553

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Remove assumption user ID is 1000 #553

wants to merge 2 commits into from

Conversation

ukkopahis
Copy link

@ukkopahis ukkopahis commented Apr 29, 2022 •
edited
Loading

DO NOT MERGE YET

On menu.sh start, store current UID and GID to .env, if they are missing.
Use these to run services that support a custom user setting.

Lines added to .env by menu.sh are e.g.:

# Changing IOTSTACK_UID or IOTSTACK_GID after you have started the stack is not
# supported. File ownerships in the 'volumes'-folder won't automatically update
# to match, resulting in various problems.
IOTSTACK_UID=1000
IOTSTACK_GID=1000

And in templates and services:

services:
  wireguard:
     environment:
     - PUID=${IOTSTACK_UID:?IOTSTACK_UID must be defined in ~/IOTstack/.env}
     - PGID=${IOTSTACK_GID:?IOTSTACK_GID must be defined in ~/IOTstack/.env}

When dropping the ID=1000 assumption, there is only one chance to get this right:

  • Starting containers with a given UID may create files in volumes/ using that UID for ownership. Later changing the container to run with a different UID may cause file-permission problems.
  • The UID chosen should be the one most convenient for the user, hence the logged in user that runs the menu.

Other small changes:

Resolves #542, Fixes #183

@ukkopahis ukkopahis force-pushed the dynamic-service-uid branch 2 times, most recently from 2fc86ca to 0b29eb3 Compare April 29, 2022 06:48
@Paraphraser
Copy link

Paraphraser commented Apr 29, 2022 •
edited
Loading

Err - 100 ? Should that be 1000 ?

I think it's only the commit message above. I couldn't find any 100 in the changed files (but I do have ancient eyeballs).

@ukkopahis
Copy link
Author

Err - 100 ? Should that be 1000 ?

I was trying to show what it does on a non-default install. Clarified.

@ukkopahis ukkopahis force-pushed the dynamic-service-uid branch from 0b29eb3 to 157cebe Compare April 29, 2022 15:48
@ukkopahis ukkopahis changed the title menu: remove assumption user ID is 1000 templates: remove assumption user ID is 1000 Apr 30, 2022
@ukkopahis ukkopahis force-pushed the dynamic-service-uid branch 5 times, most recently from 993e156 to 3fa64f7 Compare May 1, 2022 20:42
@ukkopahis ukkopahis changed the title templates: remove assumption user ID is 1000 Remove assumption user ID is 1000 May 1, 2022
@ukkopahis ukkopahis force-pushed the dynamic-service-uid branch 5 times, most recently from 8bdfdb8 to c60b752 Compare May 4, 2022 03:44
ukkopahis added a commit to ukkopahis/IOTstack that referenced this pull request May 4, 2022
@ukkopahis
duckdns: add service
c86a935 
This container includes cron and should be easier to use than the
current duck.sh script. After a test period this should completely
replace the current ./duck/duck.sh script.

Currently uses my fork of linuxserver/docker-duckdns to include
some needed pull-requests that are pending.

Depends on SensorsIot#553 to generate .env with IOTSTACK_UID and IOTSTACK_GID

Mkdocs and material versions updated to fix layout bug.
@ukkopahis ukkopahis mentioned this pull request May 4, 2022
Merged
@ukkopahis ukkopahis force-pushed the dynamic-service-uid branch 2 times, most recently from 9074543 to 27fb6c0 Compare May 9, 2022 19:37
ukkopahis added a commit to ukkopahis/IOTstack that referenced this pull request May 18, 2022
@ukkopahis
duckdns: add service
84205b6 
This container includes cron and should be easier to use than the
current duck.sh script. After a test period this should completely
replace the current ./duck/duck.sh script.

Currently uses my fork of linuxserver/docker-duckdns to include
some needed pull-requests that are pending upstream pull-requests.

Depends on SensorsIot#553 to generate .env with IOTSTACK_UID and IOTSTACK_GID

Mkdocs and material versions updated to fix layout bug.
ukkopahis added a commit to ukkopahis/IOTstack that referenced this pull request May 18, 2022
@ukkopahis
duckdns: add service
5f9e044 
This container includes cron and should be easier to use than the
current duck.sh script. After a test period this should completely
replace the current ./duck/duck.sh script.

Currently uses my fork of linuxserver/docker-duckdns to include
some needed pull-requests that are pending upstream pull-requests.

Depends on SensorsIot#553 to generate .env with IOTSTACK_UID and IOTSTACK_GID

Mkdocs and material versions updated to fix layout bug.
@ukkopahis
Copy link
Author

ukkopahis commented May 21, 2022 •
edited
Loading

Alternate suggestion (NOT RECOMMENDED)

I think it's better to change this so a .env-file isn't needed nor generated.

Templates would still use the syntax allowing .env-use:

- PUID=${IOTSTACK_UID:?replace with your actual UID}

buildstack_menu.py would identify template environment variable values matching ^\${IOTSTACK_[UG]ID:.*}$ and replace them with the actual UID/GID number of the user running the menu, resulting in a docker-compose.yml with e.g.

- PUID=1000

Resoning

  • simpler docker-compose.yml - more beginner friendly
  • restoring a backup using an user with a different UID/GID from the ones that took the backup would work better. Restored services would use the UID from backup and the existing docker-compose.yml. New added services would be using the current user's UID.
  • preserves manual usage of templates: concatenating (e.g. using cat) templates into your docker-compose.yml and adding a .env-file to define required variables.
  • same templates will work in the old menu by using a .env-file (as in the original proposal)

Drawbacks

  • more complex menu implementation needed
  • manual (template concatenation) use and menu-driven use diverge in results (manual is using .env and menu isn't)

ukkopahis added 2 commits June 17, 2022 17:58
@ukkopahis
menu: add help and cleanup
be46277
@ukkopahis
remove assumption user ID is 1000
0c6d564 
On menu.sh start, store current UID and GID to .env, if they are missing.
Use these to run services that use a customized user.
@ukkopahis ukkopahis force-pushed the dynamic-service-uid branch from 27fb6c0 to 0c6d564 Compare June 17, 2022 15:17
@ukkopahis ukkopahis mentioned this pull request Jun 24, 2022
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Don't merge yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Raspbian / RaspberryPiOS no longer allowing user Pi How to exploit docker-compose default files
3 participants
@ukkopahis @Paraphraser @Slyke